Fix missing raise keyword in _get_selector_method#465
Fix missing raise keyword in _get_selector_method#465MichaelMVS wants to merge 1 commit intoelementsinteractive:mainfrom
Conversation
The function was creating an InvalidSelectorMethodError exception but never raising it, causing the function to return None instead. This meant that invalid selector methods were silently accepted rather than raising an error as intended.
There was a problem hiding this comment.
Pull request overview
Fixes _get_selector_method to correctly raise InvalidSelectorMethodError when an unknown selector method is provided, preventing a silent None return and downstream confusing failures.
Changes:
- Add missing
raiseforInvalidSelectorMethodErrorin_get_selector_method.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _get_selector_method(selector_method: str) -> SelectorMethod: | ||
| """Return the selector_method from set of available ones.""" | ||
| if selector_method not in SELECTOR_METHOD_MAPPING: | ||
| InvalidSelectorMethodError("Invalid selector method") | ||
| raise InvalidSelectorMethodError("Invalid selector method") | ||
|
|
There was a problem hiding this comment.
The return type annotation appears incorrect: SelectorMethod (from twyn.base.constants) is a Literal[...] of method names, but this function returns an instantiated selector (SELECTOR_METHOD_MAPPING[selector_method]()), not the string key. Consider updating the return type (and related parameter annotations where this value is passed around) to the appropriate selector base type/protocol to avoid misleading typing.
| if selector_method not in SELECTOR_METHOD_MAPPING: | ||
| InvalidSelectorMethodError("Invalid selector method") | ||
| raise InvalidSelectorMethodError("Invalid selector method") |
There was a problem hiding this comment.
The raised error message is quite generic. For parity with the config validation (e.g., listing allowed selector methods), consider including the invalid value and the valid options (from SELECTOR_METHOD_MAPPING.keys()) to make CLI/config failures easier to diagnose.
Summary
The function
_get_selector_methodinsrc/twyn/main.pywas creating anInvalidSelectorMethodErrorexception but never raising it:This bug caused the function to return
Noneinstead of raising an exception when an invalid selector method was provided. The validation error was silently swallowed.Impact
When an invalid selector method is passed (either via CLI or config), instead of raising an error and informing the user, the function would return
Noneand the program would continue with aNoneselector, leading to confusing failures later in the code path.Fix
Added the missing
raisekeyword so the exception is properly raised when an invalid selector method is detected.Testing
The existing test suite passes (189 tests). This fix ensures that invalid selector methods are properly rejected with a clear error message instead of silently returning
None.